Skip to content

cfi: Remove #[no_sanitize(cfi)] for extern weak functions #139667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 17, 2025
Merged

cfi: Remove #[no_sanitize(cfi)] for extern weak functions #139667

merged 2 commits into from
Apr 17, 2025

Conversation

1c3t3a
Copy link
Member

@1c3t3a 1c3t3a commented Apr 11, 2025

Previously (#115200, #138002), we added #[no_sanitize(cfi)] to all code paths that call to a weakly linked function.

In #138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes.

r? @rcvalle

@1c3t3a
cfi: Remove #[no_sanitize(cfi)] for extern weak functions
068a332 
Previously (rust-lang#115200,
rust-lang#138002), we
added `#[no_sanitize(cfi)]` to all code paths that call to a weakly
linked function.

In rust-lang#138349 we fixed the root cause
for this issue, which means we can now remove the corresponding
attributes.
@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 11, 2025
@tamird
Copy link
Contributor

tamird commented Apr 12, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]
can also be removed.

@1c3t3a
Copy link
Member Author

1c3t3a commented Apr 13, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]
can also be removed.

True, but for that #139632 has to land first. I can wait for it to be through and then also remove this line!

@tamird
Copy link
Contributor

tamird commented Apr 13, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]

can also be removed.

True, but for that #139632 has to land first. I can wait for it to be through and then also remove this line!

Are you sure? libcore has its own:

rust/library/core/src/lib.rs

Line 172 in 53d4476

#![feature(no_sanitize)]

@rcvalle
Copy link
Member

rcvalle commented Apr 14, 2025

r? @m-ou-se

@rustbot rustbot assigned m-ou-se and unassigned rcvalle Apr 14, 2025
@rcvalle
Copy link
Member

rcvalle commented Apr 14, 2025

@m-ou-se It LGTM. Mind taking a look and r+ whenever you have time? I don't want to r+ core and std changes directly.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 15, 2025

Can you also remove #![feature(no_sanitize)] from library/std/src/lib.rs?

As @tamird mentions, that doesn't have to wait on #139632. That PR is all in library/core, while this PR is all in library/std.

@m-ou-se m-ou-se added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 15, 2025
@1c3t3a
Copy link
Member Author

1c3t3a commented Apr 15, 2025

I think

rust/library/std/src/lib.rs

Line 315 in 7cd6e2f

#![feature(no_sanitize)]

can also be removed.

True, but for that #139632 has to land first. I can wait for it to be through and then also remove this line!

Are you sure? libcore has its own:

rust/library/core/src/lib.rs

Line 172 in 53d4476

#![feature(no_sanitize)]

Yes you are right! I wasn't aware of that, it is now fixed :)

@rcvalle
Copy link
Member

rcvalle commented Apr 16, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 16, 2025
@m-ou-se
Copy link
Member

m-ou-se commented Apr 16, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 16, 2025

📌 Commit c4d3563 has been approved by m-ou-se

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 16, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
@bors
Auto merge of rust-lang#139940 - matthiaskrgr:rollup-rd4d3fn, r=matth…
15c4cce 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#135340 (Add `explicit_extern_abis` Feature and Enforce Explicit ABIs)
 - rust-lang#139440 (rustc_target: RISC-V: feature addition batch 2)
 - rust-lang#139667 (cfi: Remove #[no_sanitize(cfi)] for extern weak functions)
 - rust-lang#139828 (Don't require rigid alias's trait to hold)
 - rust-lang#139854 (Improve parse errors for stray lifetimes in type position)
 - rust-lang#139889 (Clean UI tests 3 of n)
 - rust-lang#139894 (Fix `opt-dist` CLI flag and make it work without LLD)
 - rust-lang#139900 (stepping into impls for normalization is unproductive)
 - rust-lang#139915 (replace some #[rustc_intrinsic] usage with use of the libcore declarations)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f14e632 into rust-lang:master Apr 17, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 17, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2025
@rust-timer
Unrolled build for rust-lang#139667
365e919 
Rollup merge of rust-lang#139667 - 1c3t3a:remove-no-sanitize, r=m-ou-se

cfi: Remove #[no_sanitize(cfi)] for extern weak functions

Previously (rust-lang#115200, rust-lang#138002), we added `#[no_sanitize(cfi)]` to all code paths that call to a weakly linked function.

In rust-lang#138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes.

r? `@rcvalle`
@rcvalle rcvalle mentioned this pull request Apr 18, 2025
Closed
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 19, 2025
@matthiaskrgr
Rollup merge of rust-lang#139667 - 1c3t3a:remove-no-sanitize, r=m-ou-se
9a520f8 
cfi: Remove #[no_sanitize(cfi)] for extern weak functions

Previously (rust-lang#115200, rust-lang#138002), we added `#[no_sanitize(cfi)]` to all code paths that call to a weakly linked function.

In rust-lang#138349 we fixed the root cause for this issue, which means we can now remove the corresponding attributes.

r? `@rcvalle`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rcvalle rcvalle rcvalle left review comments

Assignees

@m-ou-se m-ou-se

Labels
O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.
6 participants
@1c3t3a @tamird @rcvalle @m-ou-se @bors @rustbot